[BUGFIX] 운세 생성 상태에 만료 기한 추가하여 유령 Creating 문제 해결#257
Conversation
WalkthroughFortune 관련 상태를 UserPreferences에서 분리해 새로운 FortunePreferences로 이동하고, 운세 생성에 attemptId와 만료(leaseMillis)를 도입해 per-attempt 상태 추적 및 만료 기반 자동 교정 로직을 추가했습니다. LocalDataSource, Repository, Worker의 관련 메서드 시그니처가 attemptId/leaseMillis를 받도록 변경되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor System
participant Worker as PostFortuneWorker
participant Repo as FortuneRepository
participant LDS as FortuneLocalDataSource
participant Pref as FortunePreferences(DataStore)
System->>Worker: run()
Worker->>Worker: attemptId = UUID.randomUUID()
Worker->>Repo: markFortuneAsCreating(attemptId, leaseMillis)
Repo->>LDS: markFortuneCreating(attemptId, leaseMillis)
LDS->>Pref: set CREATING=true, ATTEMPT_ID, STARTED_AT, EXPIRES_AT
alt 성공
Worker->>Repo: markFortuneAsCreated(attemptId, fortuneId)
Repo->>LDS: markFortuneCreated(attemptId, fortuneId)
LDS->>Pref: update ID/DATE, CREATING=false, FAILED=false, reset SEEN/TOOLTIP(오늘)
Worker->>Pref: saveFortuneScore(score)
Worker-->>System: Result.success
else 실패/예외
Worker->>Repo: markFortuneAsFailed(attemptId)
Repo->>LDS: markFortuneFailed(attemptId)
LDS->>Pref: set FAILED=true, CREATING=false (attempt 일치 시)
Worker-->>System: Result.retry 또는 Result.failure
end
sequenceDiagram
participant Pref as FortunePreferences
participant UI as UI/VM
note over Pref,UI: Flow 정책: catch -> emptyPreferences(), distinctUntilChanged()\nisFortuneCreatingFlow: EXPIRES_AT < now -> 자동으로 Failure로 교정
Pref-->>UI: fortuneIdFlow, fortuneDateEpochFlow, isFortuneCreatingFlow, isFortuneFailedFlow, hasUnseenFortuneFlow, shouldShowFortuneToolTipFlow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is ❌ Your project status has failed because the head coverage (5.85%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #257 +/- ##
============================================
+ Coverage 4.06% 5.85% +1.79%
- Complexity 53 67 +14
============================================
Files 51 53 +2
Lines 4670 4847 +177
Branches 689 706 +17
============================================
+ Hits 190 284 +94
- Misses 4470 4537 +67
- Partials 10 26 +16
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (10)
data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSource.kt (1)
17-19: 시그니처 변경 방향 적절 + 계약(Contract) 명확화 제안attemptId/leaseMillis 도입은 문제 정의에 정확히 부합합니다. 향후 혼선을 막기 위해 아래를 인터페이스 KDoc에 명시해 주세요: 단위(ms), 유효 범위(>0), 시계 기준(벽시계 vs 단조 시계), idempotency(동일 attemptId 재호출 시 기대 동작).
interface FortuneLocalDataSource { @@ - suspend fun markFortuneCreating(attemptId: String, leaseMillis: Long) + /** + * 운세 생성 시도를 Creating으로 표시합니다. + * @param attemptId 시도 식별자(비어있지 않아야 함). 동일 id 재호출 시 idempotent 동작을 기대. + * @param leaseMillis ms 단위 임대기간(>0). 임대 만료 시 Creating은 자동 교정 대상이 됩니다. + * 시계 기준: System.currentTimeMillis() 기반인지(벽시계 변동 영향) 명시 필요. + */ + suspend fun markFortuneCreating(attemptId: String, leaseMillis: Long) @@ - suspend fun markFortuneCreated(attemptId: String, fortuneId: Long) + /** attemptId가 일치할 때에만 Created 전이 */ + suspend fun markFortuneCreated(attemptId: String, fortuneId: Long) @@ - suspend fun markFortuneFailed(attemptId: String) + /** attemptId가 일치할 때에만 Failed 전이 */ + suspend fun markFortuneFailed(attemptId: String)feature/fortune/src/main/java/com/yapp/fortune/worker/PostFortuneWorker.kt (2)
35-38: userId 없음 시 Result.failure() 재검토이 워커가 PeriodicWork이면 다음 주기엔 재시도되니 괜찮지만, OneTimeWork라면 로그인 후 자동 재시도가 안 됩니다. 의도대로라면
Result.retry()가 더 맞을 수 있습니다. 스케줄링 정책(Unique/Periodic/OneTime)을 확인해 주세요.
53-57: 재시도 조건 세분화 제안모든 실패를
Result.retry()로 취급하면 영구 재시도 루프가 날 수 있습니다. HTTP 4xx/도메인 불변 위반 등 비가역 오류는Result.failure()로 분기하는 에러 매핑을 고려해 주세요(예: RemoteDataSource에서 도메인 예외로 변환).data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt (1)
22-35: 상태 결합 로직 합리적 + 오늘 판정 시계 주입 고려
- Failure > Creating > Success > Idle 우선순위는 명확합니다. LGTM.
todayEpoch()가LocalDate.now()(시스템 타임존/벽시계)에 의존합니다. 테스트 용이성과 시간 변경(사용자 수동 변경/서머타임) 영향 완화를 위해Clock주입 또는 래핑을 권장합니다.-import java.time.LocalDate +import java.time.Clock +import java.time.LocalDate @@ -class FortuneLocalDataSourceImpl @Inject constructor( - private val fortunePreferences: FortunePreferences, -) : FortuneLocalDataSource { +class FortuneLocalDataSourceImpl @Inject constructor( + private val fortunePreferences: FortunePreferences, + private val clock: Clock, +) : FortuneLocalDataSource { @@ - private fun todayEpoch(): Long = LocalDate.now().toEpochDay() + private fun todayEpoch(): Long = LocalDate.now(clock).toEpochDay()DI에
Clock.systemDefaultZone()바인딩만 추가하면 됩니다.domain/src/main/java/com/yapp/domain/repository/FortuneRepository.kt (1)
18-20: 확인: 만료 후에도 ATTEMPT_ID 검사로 created/failed가 정상 반영됩니다 — Duration 사용 권장FortunePreferences의 markFortuneCreatedIfAttemptMatches / markFortuneFailedIfAttemptMatches는 ATTEMPT_ID만 비교하므로 만료된 이후에도(또는 isFortuneCreatingFlow가 만료를 교정한 이후에도) 서버 응답이 정상 반영됩니다. isFortuneCreatingFlow는 만료 시 CREATING=false·FAILED=true로 교정하지만 ATTEMPT_ID는 삭제하지 않습니다. (파일: core/datastore/src/main/java/com/yapp/datastore/FortunePreferences.kt — isFortuneCreatingFlow, markFortuneCreatedIfAttemptMatches, markFortuneFailedIfAttemptMatches)
선택적 권장: API 가독성/오용 방지를 위해 leaseMillis: Long → kotlin.time.Duration 사용 검토.
-import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.Flow +import kotlin.time.Duration +import kotlin.time.Duration.Companion.minutes @@ - suspend fun markFortuneAsCreating(attemptId: String, leaseMillis: Long = 2 * 60_000L) + suspend fun markFortuneAsCreating(attemptId: String, lease: Duration = 2.minutes)core/datastore/src/main/java/com/yapp/datastore/FortunePreferences.kt (5)
102-109: 만료/레거시 교정 시에도 시도 메타데이터 정리교정 후 ATTEMPT_ID/STARTED_AT/EXPIRES_AT가 남아 후속 로직을 혼동시킬 수 있습니다.
if (legacy || expired) { // 레거시(만료정보 없음) 또는 만료 → 실패로 교정 dataStore.edit { pref -> pref[Keys.CREATING] = false pref[Keys.FAILED] = true + // 교정 종료 → 시도 메타데이터 정리 + pref.remove(Keys.ATTEMPT_ID) + pref.remove(Keys.STARTED_AT) + pref.remove(Keys.EXPIRES_AT) } emit(false) return@transformLatest }
200-210: clearFortuneData가 시도 메타데이터를 남김사용자 데이터 초기화 시 시도 메타데이터도 함께 제거하는 편이 직관적입니다.
suspend fun clearFortuneData() { dataStore.edit { pref -> pref.remove(Keys.ID) pref.remove(Keys.DATE) pref.remove(Keys.IMAGE_ID) pref.remove(Keys.SCORE) pref.remove(Keys.SEEN) pref.remove(Keys.TOOLTIP_SHOWN) pref.remove(Keys.CREATING) pref.remove(Keys.FAILED) + // 시도 메타데이터도 정리 + pref.remove(Keys.ATTEMPT_ID) + pref.remove(Keys.STARTED_AT) + pref.remove(Keys.EXPIRES_AT) } }
50-86: DataStore 예외를 전부 삼키는 패턴: 최소한의 로깅 고려
catch { emit(emptyPreferences()) }는 안전하지만, 반복되는 오류를 파악하기 어렵습니다. 공용 연산자(예:private fun <T> Flow<Preferences>.safeMap(...))로 감싸며 로깅(디버그/에러 레벨) 추가를 고려하세요.
47-49: todayEpoch()/nowMillis() 중복 유틸 통일 제안다른 모듈에도 동일 유틸이 존재합니다(
FortuneLocalDataSourceImpl.kt,UserPreferences.kt). 공용 유틸로 추출해 시계 주입 일관성을 높이세요.
130-141: lease 파라미터를 leaseMillis로 이름 통일 — 네임드 인자 충돌 예방data 모듈의 인터페이스/구현부가 leaseMillis를 사용합니다. 저장소(FortunePreferences.kt)의 파라미터명을 일치시켜 주세요 — repo에서 명시적 네임드 호출(leaseMillis=...)은 발견되지 않았지만 일관성 유지 권장.
- suspend fun markFortuneCreating( - attemptId: String, - lease: Long, - ) { + suspend fun markFortuneCreating( + attemptId: String, + leaseMillis: Long, + ) { val now = nowMillis() dataStore.edit { pref -> pref[Keys.CREATING] = true pref[Keys.FAILED] = false pref[Keys.ATTEMPT_ID] = attemptId pref[Keys.STARTED_AT] = now - pref[Keys.EXPIRES_AT] = now + lease + pref[Keys.EXPIRES_AT] = now + leaseMillis } }대안(선택): java.time.Duration 사용으로 단위 혼동 방지.
참고 파일: data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSource.kt, data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt, core/datastore/src/main/java/com/yapp/datastore/FortunePreferences.kt
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
core/datastore/src/main/java/com/yapp/datastore/FortunePreferences.kt(1 hunks)core/datastore/src/main/java/com/yapp/datastore/UserPreferences.kt(0 hunks)data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSource.kt(1 hunks)data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt(2 hunks)data/src/main/java/com/yapp/data/repositoryimpl/FortuneRepositoryImpl.kt(1 hunks)domain/src/main/java/com/yapp/domain/repository/FortuneRepository.kt(1 hunks)feature/fortune/src/main/java/com/yapp/fortune/worker/PostFortuneWorker.kt(2 hunks)
💤 Files with no reviewable changes (1)
- core/datastore/src/main/java/com/yapp/datastore/UserPreferences.kt
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-14T15:32:44.064Z
Learnt from: DongChyeon
PR: YAPP-Github/Orbit-Android#252
File: domain/src/main/java/com/yapp/domain/repository/FortuneRepository.kt:16-17
Timestamp: 2025-09-14T15:32:44.064Z
Learning: Fortune creation requests have at least a 1-minute gap between them in the Orbit Android app, making atomic guards for race condition prevention unnecessary in the fortune creation flow.
Applied to files:
data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSource.ktdomain/src/main/java/com/yapp/domain/repository/FortuneRepository.ktdata/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt
🧬 Code graph analysis (1)
core/datastore/src/main/java/com/yapp/datastore/FortunePreferences.kt (2)
data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt (1)
todayEpoch(36-36)core/datastore/src/main/java/com/yapp/datastore/UserPreferences.kt (1)
todayEpoch(31-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
data/src/main/java/com/yapp/data/repositoryimpl/FortuneRepositoryImpl.kt (1)
27-29: 위임 구현 깔끔합니다시그니처 변경에 맞춘 단순 위임으로 일관성 좋습니다.
feature/fortune/src/main/java/com/yapp/fortune/worker/PostFortuneWorker.kt (1)
25-33: 초기 가드 OK, 단 “Creating 만료 교정”과의 레이스만 확인
Creating/Success면 조기 종료하는 정책은 타당합니다. 다만 isFortuneCreatingFlow가 만료 교정을 즉시 반영하지 못하는 타이밍에서는 조기 종료로 인해 시도가 생략될 수 있으니, 교정이 플로우에 즉시 반영됨을 확인해 주세요.data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt (2)
14-21: 위임 플로우 교체 LGTMUserPreferences → FortunePreferences 전환과 각 Flow 위임은 일관되고 변경 목적에 부합합니다.
38-48: 만료(TTL) 후 지연 응답 수용 여부 확인 — 현재 ATTEMPT_ID 일치만으로 상태 전이됨FortunePreferences의 markFortuneCreatedIfAttemptMatches / markFortuneFailedIfAttemptMatches는 pref[Keys.ATTEMPT_ID] == attemptId만 검사합니다. isFortuneCreatingFlow가 만료 시 CREATING=false, FAILED=true로 교정하지만 ATTEMPT_ID는 제거하지 않으므로, TTL 경과 후 성공 응답이 동일한 attemptId로 오면 실패로 교정된 상태가 성공으로 덮어씌워집니다.
위치: core/datastore/src/main/java/com/yapp/datastore/FortunePreferences.kt — markFortuneCreatedIfAttemptMatches / markFortuneFailedIfAttemptMatches / isFortuneCreatingFlow
권장 조치(의도에 따라 하나 선택): 1) 만료 교정 시 ATTEMPT_ID 제거, 2) markFortuneCreatedIfAttemptMatches에 CREATING==true 또는 expiresAt 검사 추가, 3) 현재 동작이 의도라면 주석/테스트로 의도 명시.
feature/fortune/src/main/java/com/yapp/fortune/worker/PostFortuneWorker.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
core/datastore/src/main/java/com/yapp/datastore/FortunePreferences.kt (2)
159-161: 이전 피드백 반영: 성공 시 시도 메타데이터 정리 확인성공 완료 후 ATTEMPT_ID/STARTED_AT/EXPIRES_AT 제거가 반영되었습니다. 굿.
171-179: 실패 마킹도 CREATING 상태에서만 진행해야 함성공 후 지연 실패 신호가 덮어쓰지 않도록 CREATING==true 가드가 필요합니다. (이전 코멘트 연장선)
적용 패치:
- if (pref[Keys.ATTEMPT_ID] == attemptId) { + if (pref[Keys.ATTEMPT_ID] == attemptId && (pref[Keys.CREATING] == true)) { pref[Keys.CREATING] = false pref[Keys.FAILED] = true pref.remove(Keys.ATTEMPT_ID) pref.remove(Keys.STARTED_AT) pref.remove(Keys.EXPIRES_AT) }feature/fortune/src/main/java/com/yapp/fortune/worker/PostFortuneWorker.kt (1)
59-61: CancellationException 재전파 반영 확인취소를 실패/재시도로 오인하지 않도록 한 처리 굿.
🧹 Nitpick comments (4)
core/datastore/src/main/java/com/yapp/datastore/FortunePreferences.kt (4)
102-111: 만료/레거시 교정 시 시도 메타데이터도 함께 정리하세요교정 후 attempt 메타데이터가 남아 있으면 추후 레이스에서 오작동 여지가 있습니다.
적용 패치:
if (legacy || expired) { // 레거시(만료정보 없음) 또는 만료 → 실패로 교정 dataStore.edit { pref -> pref[Keys.CREATING] = false pref[Keys.FAILED] = true + pref.remove(Keys.ATTEMPT_ID) + pref.remove(Keys.STARTED_AT) + pref.remove(Keys.EXPIRES_AT) } emit(false) return@transformLatest }Also applies to: 104-107
206-216: clearFortuneData에서 attempt 메타데이터 누락운세 관련 정리 시도 시 ATTEMPT_ID/STARTED_AT/EXPIRES_AT도 함께 제거해야 일관성이 유지됩니다.
적용 패치:
dataStore.edit { pref -> pref.remove(Keys.ID) pref.remove(Keys.DATE) pref.remove(Keys.IMAGE_ID) pref.remove(Keys.SCORE) pref.remove(Keys.SEEN) pref.remove(Keys.TOOLTIP_SHOWN) pref.remove(Keys.CREATING) pref.remove(Keys.FAILED) + pref.remove(Keys.ATTEMPT_ID) + pref.remove(Keys.STARTED_AT) + pref.remove(Keys.EXPIRES_AT) }
100-101: 만료 경계 비교는 >=가 안전합니다동일 시각(밀리초)에서 경계 미스 방지.
- val expired = (!legacy && nowMillis() > expiresAt) + val expired = (!legacy && nowMillis() >= expiresAt)
131-141: 파라미터 명확성: lease → leaseMillis로 명명단위 명시로 오용 방지. 호출부 영향이 크면 스킵 가능.
- suspend fun markFortuneCreating( - attemptId: String, - lease: Long, - ) { + suspend fun markFortuneCreating( + attemptId: String, + leaseMillis: Long, + ) { val now = nowMillis() dataStore.edit { pref -> @@ - pref[Keys.EXPIRES_AT] = now + lease + pref[Keys.EXPIRES_AT] = now + leaseMillis } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
core/datastore/src/main/java/com/yapp/datastore/FortunePreferences.kt(1 hunks)feature/fortune/src/main/java/com/yapp/fortune/worker/PostFortuneWorker.kt(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-14T15:32:44.064Z
Learnt from: DongChyeon
PR: YAPP-Github/Orbit-Android#252
File: domain/src/main/java/com/yapp/domain/repository/FortuneRepository.kt:16-17
Timestamp: 2025-09-14T15:32:44.064Z
Learning: Fortune creation requests have at least a 1-minute gap between them in the Orbit Android app, making atomic guards for race condition prevention unnecessary in the fortune creation flow.
Applied to files:
feature/fortune/src/main/java/com/yapp/fortune/worker/PostFortuneWorker.kt
🧬 Code graph analysis (1)
core/datastore/src/main/java/com/yapp/datastore/FortunePreferences.kt (2)
data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt (1)
todayEpoch(36-36)core/datastore/src/main/java/com/yapp/datastore/UserPreferences.kt (1)
todayEpoch(31-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
feature/fortune/src/main/java/com/yapp/fortune/worker/PostFortuneWorker.kt (2)
25-34: 중복 실행 가드 적절Creating/Success 시 조기 종료로 중복 워크 방지 OK.
41-45: lease 전달 경로 확인 — 정상검증 결과: Worker에서 markFortuneAsCreating(attemptId) 호출 시 도메인 인터페이스의 기본 leaseMillis(2 * 60_000L)가 적용되며, FortuneRepositoryImpl → FortuneLocalDataSourceImpl → FortunePreferences.markFortuneCreating(attemptId, lease)로 전달되어 EXPIRES_AT에 now + lease로 저장됩니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
core/datastore/src/test/kotlin/com/yapp/datastore/FortunePreferencesTest.kt (5)
10-13: 불리언 비교는 assertTrue/assertFalse를 사용해 가독성 향상불리언에 대한
assertEquals(true/false, …)대신assertTrue/False를 사용하면 실패 메시지가 더 명확하고 관례적입니다.적용 diff 제안:
@@ -import org.junit.Assert.assertEquals +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue @@ - assertEquals(false, creating) + assertFalse(creating) @@ - assertEquals(true, failed) + assertTrue(failed) @@ - assertEquals(false, creating) + assertFalse(creating) @@ - assertEquals(true, failed) + assertTrue(failed) @@ - assertEquals(true, stillCreating) + assertTrue(stillCreating) @@ - assertEquals(false, creatingAfterSuccess) + assertFalse(creatingAfterSuccess) @@ - assertEquals(false, failedAfterSuccess) + assertFalse(failedAfterSuccess) @@ - assertEquals(true, stillCreating) + assertTrue(stillCreating) @@ - assertEquals(false, creatingAfterFail) + assertFalse(creatingAfterFail) @@ - assertEquals(true, failed) + assertTrue(failed) @@ - assertEquals(false, unseen) + assertFalse(unseen) @@ - assertEquals(true, unseen) + assertTrue(unseen) @@ - assertEquals(false, showTooltip) + assertFalse(showTooltip) @@ - assertEquals(true, showTooltip) + assertTrue(showTooltip)Also applies to: 63-67, 82-86, 105-106, 115-118, 138-139, 145-148, 165-166, 182-183, 200-201, 217-218
38-41: DataStore scope를 명시하면 테스트 결정성이 더 좋아집니다현재
PreferenceDataStoreFactory.create { ... }의 기본 scope(IO+SupervisorJob)를 사용합니다. 테스트에서는StandardTestDispatcher(testScheduler)기반의 테스트 scope를 명시해주면 스케줄링이 일관되어 드문 타이밍 플레이키를 줄일 수 있습니다.적용 예(개략):
- 파일 상단에
private val testDispatcher = StandardTestDispatcher()정의PreferenceDataStoreFactory.create(scope = CoroutineScope(testDispatcher + SupervisorJob())) { newFile(...) }- 각 테스트는
runTest(testDispatcher) { ... }로 실행
48-67: 만료 교정 경계 테스트 추가 제안(만료 전 유지 확인)만료 이후 교정은 검증했지만, 만료 전에는 Creating이 유지됨을 보장하는 테스트가 있으면 회귀 방지에 도움이 됩니다.
예시 테스트:
@Test fun `운세_생성_상태_Creating_만료_전에는_유지된다`() = runTest { val ds = createNewDataStoreWithFile("prefs_not_expired.preferences_pb") val prefs = createFortunePreferencesWithClock(ds, fixedClockAtT0) // t0 val attemptId = UUID.randomUUID().toString() prefs.markFortuneCreating(attemptId = attemptId, lease = 60_000L) // 60초 리스 // 만료 전: Creating 유지, Failed 아님 assertTrue(prefs.isFortuneCreatingFlow.first()) assertFalse(prefs.isFortuneFailedFlow.first()) }
88-123: 만료 후 성공 신호 무시 시나리오도 추가하면 완결됩니다attemptId 일치 성공 케이스는 검증됐습니다. 여기에 “만료된 Creating 상태에서 동일 attemptId의 성공 신호가 와도 Success로 전환되지 않고 Failure 유지”를 추가하면 비정상 종료 후 지연된 콜백이 도착하는 현실 시나리오를 커버할 수 있습니다.
예시 테스트:
@Test fun `만료된_Creating_상태에서는_success_호출이_무시되고_Failure_유지된다`() = runTest { val ds = createNewDataStoreWithFile("prefs_expired_then_success.preferences_pb") val attemptId = UUID.randomUUID().toString() val prefsT0 = createFortunePreferencesWithClock(ds, fixedClockAtT0) prefsT0.markFortuneCreating(attemptId = attemptId, lease = 1_000L) // t0+2s 시점(만료 후)에서 성공 신호가 와도 교정 로직상 Failure 유지 val prefsT0Plus2 = createFortunePreferencesWithClock(ds, fixedClockAtT0Plus2Seconds) prefsT0Plus2.markFortuneCreatedIfAttemptMatches(attemptId = attemptId, fortuneId = 123L) assertFalse(prefsT0Plus2.isFortuneCreatingFlow.first()) assertTrue(prefsT0Plus2.isFortuneFailedFlow.first()) }
69-86: 레거시 키 명 하드코딩은 유지하되, 키 충돌 회피 주석 추가 권장레거시 교정을 검증하려면 하드코딩이 불가피합니다. 다만 실제 키명 변경 시 테스트 오탑을 빠르게 파악하기 위해 “레거시 스냅샷 의도”를 주석으로 남겨두면 좋습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
core/datastore/src/test/kotlin/com/yapp/datastore/FortunePreferencesTest.kt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
core/datastore/src/test/kotlin/com/yapp/datastore/FortunePreferencesTest.kt (1)
20-47: 전반적으로 테스트 시나리오 구성 훌륭합니다만료/레거시 교정/attemptId 매칭/읽음 상태/툴팁 노출까지 핵심 플로우가 잘 커버되었습니다. PR 목표(#256) 충족을 검증하는 테스트로 적절합니다.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
core/datastore/src/main/java/com/yapp/datastore/FortunePreferences.kt (1)
184-194: CREATING 가드 누락 — 성공 이후 지연 실패 신호 방지현재는 ATTEMPT_ID 일치만 보면 실패로 전환합니다. 성공 시 ATTEMPT_ID를 지우지만, 교정 경로 등 특수 케이스에서 안전하게 하려면 CREATING==true 가드까지 요구하는 편이 일관적입니다. (이 이슈는 과거에도 지적됨)
- dataStore.edit { pref -> - if (pref[Keys.ATTEMPT_ID] == attemptId) { + dataStore.edit { pref -> + if ((pref[Keys.CREATING] == true) && (pref[Keys.ATTEMPT_ID] == attemptId)) { pref[Keys.CREATING] = false pref[Keys.FAILED] = true pref.remove(Keys.ATTEMPT_ID) pref.remove(Keys.STARTED_AT) pref.remove(Keys.EXPIRES_AT) } }
🧹 Nitpick comments (8)
core/datastore/src/main/java/com/yapp/datastore/FortunePreferences.kt (5)
102-109: 만료/교정 시 시도 메타데이터도 함께 정리 필요만료/레거시 교정 시 CREATING=false, FAILED=true만 설정하고 ATTEMPT_ID/STARTED_AT/EXPIRES_AT는 남깁니다. 후속 처리 로직 혼선을 줄이기 위해 교정 시에도 시도 메타데이터를 제거하세요.
if (legacy || expired) { // 레거시(만료정보 없음) 또는 만료 → 실패로 교정 dataStore.edit { pref -> pref[Keys.CREATING] = false pref[Keys.FAILED] = true + pref.remove(Keys.ATTEMPT_ID) + pref.remove(Keys.STARTED_AT) + pref.remove(Keys.EXPIRES_AT) } emit(false) return@transformLatest }
219-230: clearFortuneData가 시도 메타데이터를 남깁니다사용자 초기화/로그아웃 등에서 남은 ATTEMPT_ID/STARTED_AT/EXPIRES_AT는 이후 흐름에 간섭할 수 있습니다. 함께 제거하세요.
dataStore.edit { pref -> pref.remove(Keys.ID) pref.remove(Keys.DATE) pref.remove(Keys.IMAGE_ID) pref.remove(Keys.SCORE) pref.remove(Keys.SEEN) pref.remove(Keys.TOOLTIP_SHOWN) pref.remove(Keys.CREATING) pref.remove(Keys.FAILED) + pref.remove(Keys.ATTEMPT_ID) + pref.remove(Keys.STARTED_AT) + pref.remove(Keys.EXPIRES_AT) }
130-141: lease 파라미터 검증 추가 제안(음수/과대값 방지)음수/과도한 lease 입력을 방치하면 즉시 만료되거나 비정상적으로 긴 임대가 설정될 수 있습니다. 최소/최대 범위를 강제하세요.
) { val now = nowMillis() dataStore.edit { pref -> pref[Keys.CREATING] = true pref[Keys.FAILED] = false pref[Keys.ATTEMPT_ID] = attemptId pref[Keys.STARTED_AT] = now - pref[Keys.EXPIRES_AT] = now + lease + val safeLease = lease.coerceIn(1_000L, 10 * 60_000L) // 1초 ~ 10분 (예시) + pref[Keys.EXPIRES_AT] = now + safeLease } }필요 시 상한값은 상수로 분리해 도메인 정책에 맞춰 주세요.
87-115: 만료 자동 교정이 ‘구독 시점’에만 실행됨isFortuneCreatingFlow는 DataStore 변경이 없으면 만료 시각 도래 후에도 재평가되지 않습니다. 장시간 실행 중인 화면에서 자동 교정이 지연될 수 있습니다. transformLatest 내에서 만료까지 delay 후 재확인하거나, expiresAt을 시간 틱 Flow와 결합하는 방식을 고려하세요.
간단한 방향:
- creating && !legacy && !expired인 경우, (expiresAt - now)만큼 delay 후 재검사 → 만료 시 교정.
- 또는 expiresAt을 방출하는 Flow와 combine하여 경과 시점을 트리거로 사용.
50-69: 에러 핸들링 가시성.catch { emit(emptyPreferences()) }로 에러를 삼키면 원인 추적이 어려워집니다. 최소한 로깅/크래시 리포팅 훅을 추가하세요.
예: .catch { e -> log.warn("DataStore read failed", e); emit(emptyPreferences()) }
Also applies to: 116-120, 121-129
core/datastore/src/test/kotlin/com/yapp/datastore/FortunePreferencesTest.kt (3)
48-67: 자정 경계 테스트 추가 제안오늘 의존 Flow가 자정 이후에도 즉시 갱신되는지 검증이 없습니다. 날짜 변화 후 hasUnseenFortuneFlow/shouldShowFortuneToolTipFlow가 재평가되는 테스트를 추가하세요(현재 구현은 갱신되지 않음).
원하시면 todayEpochTickerFlow 결합을 전제로 한 테스트 케이스 초안을 제공하겠습니다.
Also applies to: 151-180
125-149: 실패 마킹 가드 테스트 보강CREATING==true 가드가 추가되면(제안) 성공 처리 후 동일 attemptId의 지연 실패가 무시되는지 검증 테스트를 보강하세요.
예) 성공 후 markFortuneFailedIfAttemptMatches(validAttemptId) 호출 → FAILED가 false로 유지되는지 assert.
182-215: clearFortuneData 동작 테스트 추가초기화 후 ATTEMPT_ID/STARTED_AT/EXPIRES_AT까지 제거되는지 확인하는 테스트가 있으면 회귀를 막을 수 있습니다.
필요 시 테스트 스캐폴드를 드리겠습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
core/datastore/src/main/java/com/yapp/datastore/FortunePreferences.kt(1 hunks)core/datastore/src/test/kotlin/com/yapp/datastore/FortunePreferencesTest.kt(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-14T15:32:44.064Z
Learnt from: DongChyeon
PR: YAPP-Github/Orbit-Android#252
File: domain/src/main/java/com/yapp/domain/repository/FortuneRepository.kt:16-17
Timestamp: 2025-09-14T15:32:44.064Z
Learning: Fortune creation requests have at least a 1-minute gap between them in the Orbit Android app, making atomic guards for race condition prevention unnecessary in the fortune creation flow.
Applied to files:
core/datastore/src/main/java/com/yapp/datastore/FortunePreferences.kt
🧬 Code graph analysis (1)
core/datastore/src/main/java/com/yapp/datastore/FortunePreferences.kt (2)
data/src/main/java/com/yapp/data/local/datasource/FortuneLocalDataSourceImpl.kt (1)
todayEpoch(36-36)core/datastore/src/main/java/com/yapp/datastore/UserPreferences.kt (1)
todayEpoch(31-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
1 participant
Related issue 🛠
closed #256
어떤 변경사항이 있었나요?
CheckPoint ✅
PR이 다음 요구 사항을 충족하는지 확인하세요.
Work Description ✏️
isFortuneCreatingFlow에서expiresAt <= 0또는attemptId == null인 레거시 데이터를 즉시 Failure로 교정expiresAt)이 지났을 경우에도 동일하게 Failure 처리Uncompleted Tasks 😅
To Reviewers 📢
Summary by CodeRabbit